Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Logs UI] Use short timestamps in the log stream view #47042

Merged
merged 9 commits into from
Oct 17, 2019

Conversation

afgomez
Copy link
Contributor

@afgomez afgomez commented Oct 1, 2019

Summary

This PR changes the appearance of the log stream page. It removes the date value from the timestamp column, making it shorter and giving more space for the message.

When the date of two log entries is different, we add a marker to indicate this change. The date of the first visible log item is written in the header of the table.

Screenshots

Before:
Screenshot 2019-10-04 at 14 50 22

After:
Screenshot 2019-10-04 at 14 53 34

Known issues

When the user scrolls past a date marker the table header takes around a second to update. This is related to how we handle scrolling in this view. Since this also affects other parts of the view (i.e., how the minimap updates) and needs a big refactor, I would suggest to fix it in a different PR.

date-header-delay

Closes #45986.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@afgomez afgomez added v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.5.0 labels Oct 1, 2019
@afgomez afgomez requested a review from a team October 1, 2019 14:23
@afgomez afgomez self-assigned this Oct 1, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from be227dc to 039c171 Compare October 1, 2019 19:12
@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from 039c171 to 934f683 Compare October 2, 2019 08:24
@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch 2 times, most recently from 9bb2fc1 to 643f539 Compare October 2, 2019 10:38
@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from 643f539 to d3bdd0a Compare October 2, 2019 12:05
@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from d3bdd0a to 10ac82e Compare October 2, 2019 15:17
columnWidth={columnWidths[columnConfiguration.timestampColumn.id]}
data-test-subj="logColumnHeader timestampLogColumnHeader"
>
{firstVisiblePosition ? localizedDate(firstVisiblePosition.time) : 'Timestamp'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like that localizedDate is a utility function which can be used directly, by hooks, etc

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez marked this pull request as ready for review October 4, 2019 12:56
@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from 7a237c0 to 94ae431 Compare October 4, 2019 12:56
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from b766a14 to d9af56d Compare October 7, 2019 09:04
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@afgomez afgomez requested review from katrin-freihofner and a team October 7, 2019 13:27
Copy link
Contributor

@katrin-freihofner katrin-freihofner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩👍

@@ -83,13 +89,16 @@ const LogColumnHeadersWrapper = euiStyled.div.attrs({
justify-content: flex-start;
overflow: hidden;
padding-right: ${ASSUMED_SCROLLBAR_WIDTH}px;
border-bottom: ${props => props.theme.eui.euiBorderThin};
box-shadow: 0 2px 2px -1px ${props => transparentize(0.3, props.theme.eui.euiColorLightShade)};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use the euiSlightShadow mixin here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use the euiSlightShadow mixin here.

I'm not sure that can be done with the CSS-in-JS library we use. I'll give it a shot

@afgomez afgomez added v7.6.0 and removed v7.5.0 labels Oct 17, 2019
Alejandro Fernández Gómez added 9 commits October 17, 2019 12:39
Allows the consumers of the hook to specify if they want to show the
full date and time or just the time.
I tweaked the `RendererResult` type to allow returning strings as well
in the renderer of `WithLogPosition`.
Make the wrapper relative to ensure the shadow stays on top of the rows,
even when they are hovered.
@afgomez afgomez force-pushed the 45986-log-short-timestamp branch from d9af56d to 0347e3e Compare October 17, 2019 10:39
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Kerry350 Kerry350 self-requested a review October 17, 2019 14:00
@afgomez afgomez merged commit 917204a into elastic:master Oct 17, 2019
@afgomez afgomez deleted the 45986-log-short-timestamp branch October 17, 2019 14:13
afgomez pushed a commit to afgomez/kibana that referenced this pull request Oct 17, 2019
This commit changes the appearance of the log stream page. It removes the date value from the timestamp column, making it shorter and giving more space for the message of the log

When the date of two log entries is different, we add a marker to indicate this change. The date of the first visible log item is written in the header of the table.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Logs UI] timestamp visualization
5 participants